Pin command fixes #2477
Pin command fixes #2477
Conversation
|
Maybe instead of creating a new object for the unpinning, we should change the field name of the existing object to something agnostic like 'Pins' or 'Refs' |
|
How about Hash (to be consistent with the add command)? On March 15, 2016 5:23:01 PM EDT, Jeromy Johnson notifications@github.com wrote:
Steven Allen |
|
I don't think making |
| @@ -187,7 +187,7 @@ Example: | |||
| cmds.StringArg("ipfs-path", false, true, "Path to object(s) to be listed."), | |||
| }, | |||
| Options: []cmds.Option{ | |||
| cmds.StringOption("type", "t", "The type of pinned keys to list. Can be \"direct\", \"indirect\", \"recursive\", or \"all\". Defaults to \"recursive\"."), | |||
| cmds.StringOption("type", "t", "The type of pinned keys to list. Can be \"direct\", \"indirect\", \"recursive\", or \"all\". Defaults to \"all\"."), | |||
There was a problem hiding this comment.
Could we remove the 'Defaults to "All"' from the help string and add .Default("all") to the end of the StringOption constructor?
The down later where we check if typeStrFound { we can drop that check and just rely on the code to pick its own default value correctly.
There was a problem hiding this comment.
|
@whyrusleeping which change? default to all? |
|
@RichardLitt relevant to the API: the json object sent by the pin add and rm commands now has a field of simply It also corrects the documentation to say that "all" is the default instead of "recursive" (no code changes were made for this, all was already the default) |
|
@whyrusleeping Cool. Will update http api when this is merged. |
|
This looks good to me, @Stebalien could you rebase the changes on latest master? |
This way, we don't claim to pin when unpinning. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>
…ype` License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>
|
@whyrusleeping Done. |
|
@Stebalien the change to default to "all" was made following this discussion: #2208 (comment) To summarize the discussion, the reason is that I added arguments to |
|
Thanks @chriscool. |
ipfs pin lsnow defaults to "all", not "recursive".ipfs pin rmshould respond withUnpinned: HASH, notPinned: HASH. This is a breaking change.I don't know how you feel about multiple commits per pull but these were both trivial.